Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Password salting support #150

Merged
merged 8 commits into from
Aug 26, 2024
Merged

Password salting support #150

merged 8 commits into from
Aug 26, 2024

Conversation

inetol
Copy link
Contributor

@inetol inetol commented Jul 31, 2024

Closes #148

@Mrgaton
Copy link
Member

Mrgaton commented Jul 31, 2024

Salt no deveria ser aleatorio por cada hash? como el iv en aes

@inetol
Copy link
Contributor Author

inetol commented Jul 31, 2024

Sí, pero no tengo base de datos para almacenar los salts todavía por eso lo he hecho de esta forma.

@Mrgaton
Copy link
Member

Mrgaton commented Jul 31, 2024

No hay necesidad de hacer eso puedes hacer lo mismo que con aes, generar el salt y almacenarlo al final del salt

@Mrgaton
Copy link
Member

Mrgaton commented Jul 31, 2024

Luego cuando lo verifiques sacas los últimos bytes y ese es el hash

@inetol
Copy link
Contributor Author

inetol commented Jul 31, 2024

La gracia de tener el salt es no almacenarlo en el mismo documento donde se encriptan los datos porque entonces no sirve para nada, si se llegase a obtener una copia en crudo del documento (directamente de la carpeta de documents) el salt estaría a simple vista

@Mrgaton
Copy link
Member

Mrgaton commented Jul 31, 2024

Para eso necesitarian aceso a los archivos del servidor y si lo tienen es tan facil como copiar si no .env

@inetol
Copy link
Contributor Author

inetol commented Jul 31, 2024

Los .env se pueden blindar ya usando los secrets de Docker/Podman o algún otro sistema como AWS Secrets Manager. El problema son los documentos que almacenan los hashes porque son vulnerables a rainbow tables y revelan la reutilización de contraseñas/secretos, si se llega a filtrar algún documento en crudo desde un endpoint, volumen (o en un futuro S3) los datos y contraseñas quedan vulnerables.

La solución de almacenar un salt global en el .env ya no me parece tan buena idea (porque revela la reutilización de contraseñas) y prefiero esperar cuando esté listo la base de datos para retomar este PR.

@Mrgaton
Copy link
Member

Mrgaton commented Jul 31, 2024

Las Rainbow tables no sirven para nada si el salt es aleatorio cada vez, y el salt se podría añadir al final del hash como en aes que al final hay un Contact del buffer encriptado más el IV

@Mrgaton
Copy link
Member

Mrgaton commented Aug 1, 2024

Si quieres pruebo a implementarlo de esa forma y vemos.

@inetol
Copy link
Contributor Author

inetol commented Aug 1, 2024

O sea si está bien eso, aunque no protege de todos los casos.

Lo que veo es que si alguien pone de contraseña "a" este seguiría pudiéndose saber con un ataque de diccionario o fuerza bruta porque solo tendrías que extraer el salt del hash y empezar a comparar. Para darle más utilidad al salt lo que suelo hacer es almacenarlo en otro sitio, así si alguien tuviese el fichero en crudo del documento no solo necesitarías saber la contraseña, sino saber también el propio salt, ya luego depende el nivel de paranoia de cada uno complicar a niveles poco sanos este proceso.

@Mrgaton
Copy link
Member

Mrgaton commented Aug 1, 2024

Lo que tu pides aparte del salt es un hmac que hay si que deveria ser un secret para .env, el salt esta echo solo para que con el mismo input puedan haber multiples outputs

@Mrgaton
Copy link
Member

Mrgaton commented Aug 1, 2024

Lo malo de esto es que todos los documentos que hayan sido creados con contraseña no se podran acceder nunca jamas

@Mrgaton Mrgaton marked this pull request as ready for review August 2, 2024 09:58
@Mrgaton
Copy link
Member

Mrgaton commented Aug 2, 2024

Si algo no os gusta decirlo y lo intento cambiar.

@inetol inetol added the ack:not-ready Do NOT merge yet label Aug 2, 2024
@inetol inetol changed the base branch from dev to db August 26, 2024 19:50
# Conflicts:
#	bun.lockb
#	src/server.ts
@inetol inetol removed the ack:not-ready Do NOT merge yet label Aug 26, 2024
@inetol inetol merged commit c459316 into db Aug 26, 2024
1 check was pending
@inetol inetol deleted the salting branch August 26, 2024 19:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Salted passwords
2 participants